Skip to content

Potential fix for code scanning alert no. 2: Use of insecure HostKeyCallback implementation#3

Merged
aissat merged 1 commit into
mainfrom
alert-autofix-2
Mar 23, 2026
Merged

Potential fix for code scanning alert no. 2: Use of insecure HostKeyCallback implementation#3
aissat merged 1 commit into
mainfrom
alert-autofix-2

Conversation

@aissat
Copy link
Copy Markdown
Owner

@aissat aissat commented Mar 23, 2026

Potential fix for https://github.com/aissat/sysfig/security/code-scanning/2

In general, the fix is to replace gossh.InsecureIgnoreHostKey() with a HostKeyCallback that actually verifies the server’s host key against a trusted set (an allow-list). The most straightforward secure pattern with golang.org/x/crypto/ssh is to load one or more known host public keys and use ssh.FixedHostKey (for a single key) or a custom callback that checks the key against a map of allowed keys.

Within buildSSHClientConfig in internal/core/remote_deploy.go, we should introduce a small helper that loads a trusted host public key from a file path specified via an environment variable (or similar), parse it with gossh.ParsePublicKey, and then set HostKeyCallback to gossh.FixedHostKey(publicKey). This keeps the existing authentication behaviour intact (same authMethods) and only changes host verification from “accept all” to “accept only this trusted key”. Because we’re constrained to the shown snippet and should avoid large behavioural changes, using a single optional allow-listed key is the least invasive: if the env var is set and the key loads successfully, we use FixedHostKey; otherwise we can fail early with an error, rather than silently falling back to insecure behaviour.

Concretely:

  • Add a new helper function in this file, above buildSSHClientConfig, e.g. func loadHostKeyCallback() (gossh.HostKeyCallback, error).
  • In that helper, read a host public key file path from an environment variable like SYSFIG_SSH_HOST_KEY (name is arbitrary but descriptive; we can introduce it here), use os.ReadFile and gossh.ParsePublicKey, and return gossh.FixedHostKey(pubKey).
  • Update buildSSHClientConfig to call loadHostKeyCallback; if it returns an error, propagate it; if it returns nil callback (e.g. env not set), return an error indicating that host verification is not configured.
  • Replace the direct gossh.InsecureIgnoreHostKey() use on line 435 with the previously obtained hostKeyCallback.

This keeps imports unchanged (we already import gossh and os) and maintains the current authentication options while making host key verification secure and explicit.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…allback implementation

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@aissat aissat marked this pull request as ready for review March 23, 2026 10:14
@aissat aissat merged commit 2224a02 into main Mar 23, 2026
5 checks passed
aissat added a commit that referenced this pull request May 21, 2026
…a new FileVault for encryption; introduce filterRecords function for better record filtering

Fix #1 — git wrapper deduplication: Deleted syncGitBareRun and syncGitBareOutput from sync.go; all 5 call sites now use gitBareRun/gitBareOutput from git.go.

Fix #6 — BranchFor function: Added BranchFor(rec *types.FileRecord) string to git.go. Replaced 6 inline branch-construction patterns in status.go, apply.go, diff.go, and remote_deploy.go. Also corrects a latent bug: diff.go was using "track/" for remote files; BranchFor now correctly uses "remote/".

Fix #5 — filterRecords function: Created filter.go with the shared ID+Tag+Path filter. Replaced ~30 lines of duplicated map-building and loop filtering in apply.go and remote_deploy.go.

Fix #3 — FileVault: Created vault.go with FileVault.Encrypt/Decrypt. Replaced the three-step NewKeyManager → Load → cipher pattern in track.go, apply.go, sync.go (×2), and remote_deploy.go.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant